Replace runtime codegen with functions defined in Peep.EventHandler#69
Replace runtime codegen with functions defined in Peep.EventHandler#69rkallos merged 1 commit intorkallos:mainfrom
Peep.EventHandler#69Conversation
|
This fails as tests haven't been updated yet. |
ea7ce1c to
b0f0acf
Compare
|
Tests are updated. |
rkallos
left a comment
There was a problem hiding this comment.
Looks good at a first glance.
I had dreams around extending Peep.Codegen to define functions for each metric so that busier metrics would appear in flame graphs created with perf, but I never got around to implementing it. Perhaps it might come back at some point in the future, or some clever individual will find another approach.
| assert match?(%{statsd_state: nil}, :sys.get_state(pid)) | ||
| end | ||
|
|
||
| test "a worker with non-empty global_tags applies to all metrics" do |
There was a problem hiding this comment.
Am I right in thinking that this test was removed because Peep.get_metric/3 was not modified to have global tags added?
Since that function is only really used in tests, I have half a mind to replace it with a helper function that asserts many things about metrics with a single call to the 'blessed' Peep.get_all_metrics/1. If I make that change before merging your PR, you should be able to rebase and keep this test around.
There was a problem hiding this comment.
Yes, that is the reason. Instead of this test I have added new in exporters to check if the exported data contains expected values.
b0f0acf to
16aac9c
Compare
This puts all handler code in
Peep.EventHandlerand also merges metadata during export, not during insertion. This should speedup inserting metrics by avoiding calls toMapfunctions.This also adds support for upcoming
telemetrychange where:tagsattribute inTelemetry.Metriccan be a function that takes metadata and returns changed metadata (this is meant to deprecate:tag_valuescallback).Close #66